[DO NOT MERGE] Clique Table and Preprocessing#627
[DO NOT MERGE] Clique Table and Preprocessing#627akifcorduk wants to merge 64 commits intoNVIDIA:mainfrom
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a clique-based presolve subsystem and integrates it into the MIP pipeline, extends CSR sparse-matrix API, exposes a new problem setter for host-derived constraints, adjusts build files, and updates presolve/solver/local-search flows and comments. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@akifcorduk is this still relevant? |
aliceb-nv
left a comment
There was a problem hiding this comment.
Thanks a lot for your awesome work Akif!! This is going to be super helpful in closing the optimality gap.
Just a few minor comments, mostly relevant for possible follow ups, otherwise LGTM
| std::unordered_set<i_t> clique_table_t<i_t, f_t>::get_adj_set_of_var(i_t var_idx) | ||
| { | ||
| std::unordered_set<i_t> adj_set; | ||
| for (const auto& clique_idx : var_clique_map_first[var_idx]) { | ||
| adj_set.insert(first[clique_idx].begin(), first[clique_idx].end()); | ||
| } | ||
|
|
||
| for (const auto& addtl_clique_idx : var_clique_map_addtl[var_idx]) { | ||
| adj_set.insert(first[addtl_cliques[addtl_clique_idx].clique_idx].begin(), | ||
| first[addtl_cliques[addtl_clique_idx].clique_idx].end()); | ||
| } | ||
|
|
||
| for (const auto& adj_vertex : adj_list_small_cliques[var_idx]) { | ||
| adj_set.insert(adj_vertex); | ||
| } | ||
| // Add the complement of var_idx to the adjacency set | ||
| i_t complement_idx = (var_idx >= n_variables) ? (var_idx - n_variables) : (var_idx + n_variables); | ||
| adj_set.insert(complement_idx); | ||
| adj_set.erase(var_idx); | ||
| return adj_set; |
There was a problem hiding this comment.
If i understand correctly, all adjacency sets are of size O(n_vars), which should be small enough in most cases to be a non-factor. Do you think we could use bitsets here instead? It seems to me a lot of clique logic can be translated to bitwise ops
There was a problem hiding this comment.
Yes definitely. In cuts PR, computing clique lifting with dynamic programming is done with bitsets. I think we can also convert them. But let's do it in a follow up PR as this PR is quite long.
| std::vector<addtl_clique_t<i_t, f_t>> addtl_cliques; | ||
| // TODO figure out the performance of lookup for the following: unordered_set vs vector | ||
| // keeps the indices of original(first) cliques that contain variable x | ||
| std::vector<std::unordered_set<i_t>> var_clique_map_first; |
There was a problem hiding this comment.
side note: If we ever find that we're bottlenecked on many-cliques instances, I found this paper on sparse bitmaps which looks pretty cool :) https://roaringbitmap.org/
There was a problem hiding this comment.
Thank you! There are some heuristic limits to sizes for memory and runtime but we can explore the compressing options :)
| return true; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Should this be done in the var_idx2 -> var_idx1 direction as well? The additional clique structures aren't symmetrical as I understand
There was a problem hiding this comment.
Great catch! Yes additional cliques are not symmetrical. Fixed it.
| adj_set.insert(first[addtl_cliques[addtl_clique_idx].clique_idx].begin(), | ||
| first[addtl_cliques[addtl_clique_idx].clique_idx].end()); |
There was a problem hiding this comment.
This adds the entire base clique. Should this take "start_pos_on_clique" instead to do as "check_adjacency" does? (addtl clique is: vertex_idx + first[clique_idx][start_pos_on_clique:])
There was a problem hiding this comment.
Great catch! You are right, we are inserting the whole first clique, we should only insert a representative vertex and part of first clique.
| clique_table_t<i_t, f_t>& clique_table, | ||
| dual_simplex::user_problem_t<i_t, f_t>& problem, | ||
| dual_simplex::csr_matrix_t<i_t, f_t>& A, | ||
| cuopt::timer_t& timer) |
There was a problem hiding this comment.
My intuition is that this could be significantly sped up with some bitwise ops (AND intersections, popcounts). But let's just keep this in mind for possible follow ups if this proves to be a bottleneck :)
| void problem_t<i_t, f_t>::set_constraints_from_host_user_problem( | ||
| const cuopt::linear_programming::dual_simplex::user_problem_t<i_t, f_t>& user_problem) | ||
| { | ||
| raft::common::nvtx::range fun_scope("set_constraints_from_host_user_problem"); | ||
| cuopt_assert(user_problem.handle_ptr == handle_ptr, "handle mismatch"); | ||
| cuopt_assert(user_problem.num_cols == n_variables, "num cols mismatch"); | ||
| n_constraints = user_problem.num_rows; | ||
| cuopt_assert(user_problem.rhs.size() == static_cast<size_t>(n_constraints), "rhs size mismatch"); | ||
| cuopt_assert(user_problem.row_sense.size() == static_cast<size_t>(n_constraints), | ||
| "row sense size mismatch"); | ||
| cuopt_assert(user_problem.range_rows.size() == user_problem.range_value.size(), | ||
| "range rows/value size mismatch"); | ||
|
|
There was a problem hiding this comment.
clique_table.cu can perform variable fixings as well (in remove_dominated_cliques), should the variable bounds be copied as well?
There was a problem hiding this comment.
Yes, that's correct. Initially variable fixings were not implemented, now we need to update the variable bounds too. Thanks for catching that!
This PR implements the first part of the paper from the paper: Preprocessing and Cutting Planes with Conflict Graphs.
This part contains only the preprocessing parts and clique cuts will follow in a separate PR:
The data structures and query functions are implemented and will be used as a basis for clique cuts and usage in heuristics.
Benchmark results: there is little to no change in benchmarks. This PR is a basis for the clique cuts PR.
Summary by CodeRabbit
New Features
Improvements